Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(riverpod_lint): correctly detects nested ref invocations #3805

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

josh-burton
Copy link
Contributor

@josh-burton josh-burton commented Oct 23, 2024

RefInvocations that take as a parameter another RefInvocation are now correctly detected by the analyzer.

Details

In this situation: ref.read(dep2(ref.read(dep)));

The second provider dependency (dep) is not currently detected by analyzer and the provider_dependency lints.

Related Issues

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).

  • I have updated the CHANGELOG.md of the relevant packages.
    Changelog files must be edited under the form:

    ## Unreleased fix/major/minor
    
    - Description of your change. (thanks to @yourGithubId)
  • If this contains new features or behavior changes,
    I have updated the documentation to match those changes.

Summary by CodeRabbit

  • New Features

    • Enhanced capabilities for analyzing nested provider invocations, improving accuracy in detecting complex provider compositions.
    • Introduced new provider functions and dependencies for better handling of family providers.
  • Bug Fixes

    • Fixed issues with provider_dependencies to correctly detect nested reference invocations.
    • Improved analyzer's capability to detect nested RefInvocations used as parameters.
  • Tests

    • Added multiple test cases to ensure accurate analysis of nested ref.read and ref.watch functionalities.
    • Updated existing tests to reflect changes in expected behavior.
  • Documentation

    • Updated changelogs for riverpod_lint and riverpod_analyzer_utils to reflect new features and fixes.

Copy link

coderabbitai bot commented Oct 23, 2024

Walkthrough

The pull request introduces enhancements to the riverpod_analyzer_utils and riverpod_lint packages, focusing on improved handling of method invocations and nested reference detection within the Riverpod library. Key modifications include updates to the ResolvedRiverpodLibraryResult class and the _ParseRefInvocationMixin mixin for better parsing of provider invocations. Additionally, new tests have been added to ensure accurate analysis of ref.read and ref.watch functionalities. The changelog for both packages has been updated to reflect these changes, including fixes and enhancements related to provider dependencies.

Changes

File Change Summary
packages/riverpod_analyzer_utils/lib/src/riverpod_ast/resolve_riverpod.dart - Enhanced ResolvedRiverpodLibraryResult and _ParseRefInvocationMixin for improved handling of method invocations.
- Updated visitInstanceCreationExpression to process specific expressions recursively.
packages/riverpod_analyzer_utils_tests/test/ref_invocation_test.dart - Added tests for decoding nested ref.read and ref.watch invocations with family providers, ensuring accurate analysis of complex provider interactions.
packages/riverpod_lint/CHANGELOG.md - Updated changelog to include an unreleased fix for provider_dependencies, enhancing detection of nested reference invocations and documenting version updates.
packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.dart - Introduced new functions dep3 and onFamilyDep, enhancing the provider system's capabilities.
packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.diff - Updated various function annotations to specify or remove dependencies, reflecting a consistent effort in managing provider dependencies.
packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.g.dart - Added new providers and classes, including dep3Provider and onFamilyDepProvider, with associated hash functions and management logic.
packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies_test.dart - Modified a test case to adjust the expected length of the errors list from 10 to 11, indicating an update in the expected linting results.
packages/riverpod_analyzer_utils/CHANGELOG.md - Updated changelog to include an unreleased fix for detecting nested RefInvocations, improving error detection for complex provider compositions.

Possibly related PRs

  • Support flutter_hooks 0.21.x #3775: Enhancements in handling method invocations related to references, potentially relevant to improvements in the hooks_riverpod package.
  • Support stable #3796: Updates regarding parsing capabilities for provider declarations, which may relate to changes ensuring compatibility with stable versions, improving overall functionality in the Riverpod ecosystem.

🐇 In the meadow where providers play,
New functions hop and dance today.
With nested reads and watches so bright,
Our Riverpod's growing, what a delight!
Bugs are chased away with glee,
Join the fun, come hop with me! 🌼✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 5f2bc77 and 5d02a8b.

📒 Files selected for processing (2)
  • packages/riverpod_analyzer_utils/lib/src/riverpod_ast/resolve_riverpod.dart (2 hunks)
  • packages/riverpod_analyzer_utils_tests/test/ref_invocation_test.dart (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/riverpod_analyzer_utils/lib/src/riverpod_ast/resolve_riverpod.dart
🔇 Additional comments (3)
packages/riverpod_analyzer_utils_tests/test/ref_invocation_test.dart (3)

414-482: LGTM! Comprehensive test coverage for nested ref.read invocations.

The test case thoroughly validates the analyzer's ability to detect nested ref.read invocations, including:

  • Single-level nesting: ref.read(dep2(ref.read(dep)))
  • Double-level nesting: ref.read(dep3(ref.read(dep2(ref.read(dep)))))
  • Proper resolution of provider elements
  • Accurate source text reconstruction

648-716: LGTM! Thorough test coverage for nested ref.watch invocations.

The test case effectively mirrors the ref.read tests, ensuring that watch-specific behavior is correctly handled with:

  • Single-level nesting: ref.watch(dep2(ref.watch(dep)))
  • Double-level nesting: ref.watch(dep3(ref.watch(dep2(ref.watch(dep)))))
  • Proper provider element resolution

718-763: LGTM! Good coverage of mixed ref.watch and ref.read scenarios.

The test case validates the analyzer's ability to handle mixed operations:

  • Correctly identifies both watch and read invocations
  • Verifies proper count of each invocation type
  • Ensures accurate provider element resolution

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (4)
packages/riverpod_annotation/CHANGELOG.md (1)

3-3: Consider expanding the changelog description for better clarity.

While the current description captures the basic fix, it would be more helpful to developers if it included:

  • The specific problem that was fixed (e.g., what issues occurred with nested ref invocations)
  • The impact on developers (e.g., how this improves error detection)
  • Any potential breaking changes or migration notes if applicable

Example of a more detailed entry:

- RefInvocations as a parameter of another RefInvocation are now correctly detected (thanks to @josh-burton)
+ Fixed analyzer to correctly detect nested RefInvocations when used as parameters (e.g., ref.watch(provider(ref.watch(...)))). This improves the accuracy of the analyzer's error detection for complex provider compositions. (thanks to @josh-burton)
packages/riverpod_lint/CHANGELOG.md (1)

1-3: Consider enhancing the changelog description.

While the entry is well-structured and follows the changelog format, consider making it more specific about the nested ref invocations detection:

- provider_dependencies now detects a dependency when it is a parameter of another dependency (thanks to @josh-burton)
+ provider_dependencies now correctly detects nested ref invocations where a dependency is used as a parameter of another dependency (thanks to @josh-burton)
packages/riverpod_analyzer_utils_tests/test/ref_invocation_test.dart (1)

414-414: Consider a more descriptive test name.

The current test name "Decodes ref.read family usages" could be more specific about testing nested ref invocations, which is the main focus of this PR.

-  testSource('Decodes ref.read family usages',
+  testSource('Decodes nested ref.read invocations with family providers',
packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.g.dart (1)

176-182: Deprecation notice for Dep3Ref mixin

The Dep3Ref mixin is marked as deprecated and will be removed in version 3.0. Ensure that any dependent code is updated to use Ref instead to prepare for future updates.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 5a513c5 and 5fa9131.

📒 Files selected for processing (8)
  • packages/riverpod_analyzer_utils/lib/src/riverpod_ast/resolve_riverpod.dart (1 hunks)
  • packages/riverpod_analyzer_utils_tests/test/ref_invocation_test.dart (1 hunks)
  • packages/riverpod_annotation/CHANGELOG.md (1 hunks)
  • packages/riverpod_lint/CHANGELOG.md (1 hunks)
  • packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.dart (2 hunks)
  • packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.diff (9 hunks)
  • packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.g.dart (2 hunks)
  • packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies_test.dart (1 hunks)
🧰 Additional context used
🔇 Additional comments (18)
packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies_test.dart (2)

17-17: LGTM! Error count update aligns with enhanced RefInvocation detection.

The increment in expected error count from 10 to 11 is consistent with the PR's objective to improve nested RefInvocation detection.


17-17: Verify the new error case is documented in golden files.

Please ensure the additional error case is properly documented in the golden diff file at 'test/lints/provider_dependencies/provider_dependencies.diff'.

✅ Verification successful

Let me try to locate the golden files using a different approach.


Let me check the actual test file and its golden file to verify the error cases.


The golden files are properly documented and up-to-date

The test file provider_dependencies_test.dart expects 11 errors, and the golden diff file provider_dependencies.diff correctly documents all 11 cases with their corresponding fixes:

  1. Adding dependencies for plain annotation
  2. Adding dependencies for custom annotation
  3. Adding dependencies with trailing comma
  4. Updating empty dependencies list
  5. Updating dependencies for multiple deps
  6. Removing extra dependencies
  7. Updating dependencies for family deps
  8. Removing unnecessary dependencies for noDep
  9. Removing dependencies when no deps needed (dependencies first)
  10. Removing dependencies when no deps needed (no params)
  11. Removing dependencies when no deps needed (without comma)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if the golden diff file contains documentation for the new error case
# Expected: The diff file should show changes reflecting the additional error case

# Test: Display the golden diff file content
cat test/lints/provider_dependencies/provider_dependencies.diff

# Test: Check if the file was recently modified along with this PR
git log -p test/lints/provider_dependencies/provider_dependencies.diff

Length of output: 470


Script:

#!/bin/bash
# Search for any .diff files in the repository
fd -e diff

# Search for any golden files or test files related to provider_dependencies
fd -t f provider_dependencies

# Check the test file content to understand the expected behavior
cat packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies_test.dart

Length of output: 3362


Script:

#!/bin/bash
# Check the test file content
cat packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.dart

# Check the golden diff file content
cat packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.diff

# Check the lint implementation to understand expected error cases
cat packages/riverpod_lint/lib/src/lints/provider_dependencies.dart

Length of output: 12709

packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.dart (2)

12-13: LGTM: Consistent family provider implementation.

The new dep3 provider follows the established pattern and correctly implements a family provider with an integer parameter.


76-86: Verify test case implementation.

A few observations about this test case:

  1. The dependencies array only includes dep3 but the implementation also uses dep2Provider
  2. The implementation uses ref.read instead of the recommended ref.watch

Given this is a test file, please confirm if these potential issues are intentional test cases for the linter.

Let's check if there are related test assertions:

✅ Verification successful

Let me gather more context about this test file to understand if these are intentional test cases.


Let me check the lint rule implementation to understand its purpose better.


Based on the gathered context, I can now provide the final response:

Test case implementation is intentional and correct

The onFamilyDep provider is part of a test file that verifies the provider_dependencies lint rule. The apparent issues you noticed are intentionally crafted to trigger the lint warning:

  1. The missing dep2 dependency is intentional - it tests the lint rule's ability to detect incomplete dependencies
  2. The use of ref.read instead of ref.watch is also part of the test case
  3. The expect_lint: provider_dependencies comment explicitly marks this as a test case

The lint rule's purpose is to ensure that providers properly declare their dependencies, especially when using scoped providers, as confirmed by the rule's implementation message: "If a provider depends on providers which specify 'dependencies', they should themselves specify 'dependencies' and include all the scoped providers."

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for test assertions related to onFamilyDep
rg -A 5 "onFamilyDep" --type dart

Length of output: 2604


Script:

#!/bin/bash
# Look for test file description and related test cases
rg -B 5 -A 10 "provider_dependencies" --type dart

# Also check for any expect_lint comments to understand test patterns
rg "expect_lint: provider_dependencies" --type dart -B 2 -A 2

Length of output: 45440


Script:

#!/bin/bash
# Look for the lint rule implementation
rg -A 10 "class ProviderDependencies" --type dart

Length of output: 1141

packages/riverpod_annotation/CHANGELOG.md (1)

1-3: LGTM! The changelog entry follows the standard format.

The entry properly credits the contributor and is placed in the correct section.

packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.diff (5)

Line range hint 3-34: LGTM: Dependencies correctly added to providers

The changes appropriately add dependency declarations to providers that were previously missing them, while maintaining the correct syntax and preserving other parameters.


Line range hint 39-76: LGTM: Dependencies correctly updated

The changes appropriately update the dependency lists to match the actual dependencies used in the providers.


Line range hint 93-142: LGTM: Control cases for dependency validation

These changes appropriately remove dependencies to serve as negative test cases, maintaining different annotation styles and parameters. This provides good coverage for testing the linter's behavior.


77-89: Key test case for nested ref invocations

This test case is particularly important as it verifies the PR's main objective: correctly detecting nested ref invocations. The provider uses ref.read(dep3Provider(ref.read(dep2Provider))), which is exactly the nested case we want to validate.

Let's verify if there are similar patterns in the codebase that this fix would help with:

#!/bin/bash
# Search for nested ref.read/watch patterns
ast-grep --pattern 'ref.read($a($b.read($c)))'
ast-grep --pattern 'ref.watch($a($b.read($c)))'

Line range hint 1-142: Verify test coverage for edge cases

While the test cases cover the main scenarios, let's verify if we have sufficient coverage for all edge cases.

✅ Verification successful

Based on the results, I can now generate the final response since we have comprehensive information about the test coverage.

Test coverage for provider dependencies is comprehensive

The test coverage for provider dependencies is thorough and includes all edge cases:

  • Main test file provider_dependencies.dart covers:
    • Plain annotations with/without dependencies
    • Custom annotations with various parameter combinations
    • Multiple dependencies scenarios
    • Empty dependencies lists
    • Family dependencies
    • Dependencies ordering cases
    • Removal of unnecessary dependencies

Additional coverage found in:

  • dependencies.dart covers scoped providers and generated providers
  • dependencies2.dart covers private/public provider combinations
  • Integration tests in the generator package cover complex dependency chains
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Look for other test files that might contain provider dependency tests
fd -e dart -e diff . -x grep -l "provider_dependencies"

# Look for actual usage patterns in the main codebase that might need test coverage
ast-grep --pattern '@Riverpod(dependencies: [$$$])'

Length of output: 7784

packages/riverpod_analyzer_utils_tests/test/ref_invocation_test.dart (1)

431-448: LGTM! Comprehensive test coverage for nested ref invocations.

The test assertions thoroughly verify:

  1. The total number of ref.read invocations (2)
  2. The source and provider elements for both the outer and inner invocations
  3. The correct parsing of the nested structure

This aligns well with the PR objective to fix the detection of nested RefInvocations.

packages/riverpod_analyzer_utils/lib/src/riverpod_ast/resolve_riverpod.dart (2)

113-118: Enhancement to handle nested InvocationExpression arguments

The added loop correctly processes InvocationExpression arguments within a refInvocation, enhancing the recursive parsing capability of method invocations related to references. This improvement aligns with the objective of accurately detecting nested RefInvocation instances.


113-118: Verify performance impact with deeply nested structures

While this change improves parsing, please ensure that it does not introduce significant performance overhead when dealing with deeply nested InvocationExpression structures.

packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.g.dart (5)

41-42: Hash function _dep3Hash is correctly defined

The hash function for the dep3 provider is correctly implemented.


64-106: Dep3Family class is correctly implemented

The Dep3Family class properly extends Family<int> and includes the necessary overrides for call, getProviderOverride, and dependency properties.


108-190: Dep3Provider class handles parameterization and overrides correctly

The Dep3Provider class correctly manages the parameter, overrides methods like overrideWith, createElement, operator ==, and implements a robust hashCode method using _SystemHash.


298-316: onFamilyDepProvider dependency setup is correct

The onFamilyDepProvider is correctly defined with dep3Provider included in its dependencies and transitive dependencies, aligning with the goal of detecting nested RefInvocations.


43-62: ⚠️ Potential issue

Verify license compliance for copied _SystemHash class

The _SystemHash class is copied from the Dart SDK, as indicated by the comment /// Copied from Dart SDK. Please ensure that this usage complies with the Dart SDK's licensing terms. If required, include appropriate license notices or attributions in your project.

@josh-burton josh-burton force-pushed the nested-ref-invocation branch 5 times, most recently from 2361638 to db86c1c Compare October 23, 2024 23:31
RefInvocations that take as a parameter another RefInvocation are now correctly detected by the analyzer
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (4)
packages/riverpod_analyzer_utils/CHANGELOG.md (1)

1-4: LGTM! Consider adding a date placeholder.

The changelog entry accurately describes the fix for nested RefInvocations detection. For consistency with other entries in this changelog, consider adding a date placeholder.

-## Unreleased fix
+## Unreleased fix - TBD
packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.diff (3)

Line range hint 3-33: LGTM! Test cases effectively cover basic dependency specifications.

The test cases appropriately verify the lint's behavior for adding dependencies in various annotation formats. Consider adding a test case for nested provider dependencies (e.g., ref.read(dep2(ref.read(dep)))) to directly test the scenario mentioned in the PR objectives.


Line range hint 93-147: LGTM! Good coverage of dependency removal scenarios.

The test cases effectively verify various ways of removing dependencies while maintaining other annotation parameters. Consider adding a comment explaining why dependencies are being removed in these cases to make the test intentions clearer.


Line range hint 3-147: Consider standardizing annotation formats.

While the varying formats help test lint robustness, consider standardizing:

  • Use of trailing commas
  • Multi-line vs single-line annotations
  • Capitalization of @riverpod/@Riverpod
    This would make the tests more maintainable while still testing the core functionality.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 5fa9131 and 5f2bc77.

📒 Files selected for processing (8)
  • packages/riverpod_analyzer_utils/CHANGELOG.md (1 hunks)
  • packages/riverpod_analyzer_utils/lib/src/riverpod_ast/resolve_riverpod.dart (2 hunks)
  • packages/riverpod_analyzer_utils_tests/test/ref_invocation_test.dart (2 hunks)
  • packages/riverpod_lint/CHANGELOG.md (1 hunks)
  • packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.dart (2 hunks)
  • packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.diff (10 hunks)
  • packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.g.dart (2 hunks)
  • packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies_test.dart (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/riverpod_analyzer_utils/lib/src/riverpod_ast/resolve_riverpod.dart
  • packages/riverpod_lint/CHANGELOG.md
  • packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies_test.dart
🔇 Additional comments (11)
packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.dart (2)

12-14: LGTM! Base provider implementation is correct.

The new parameterized provider follows the established pattern and is properly configured for testing nested ref invocations.


76-86: Verify this test case matches the PR objective.

This test case correctly validates the analyzer's ability to detect nested ref invocations by:

  1. Using dep3Provider with a parameter from dep2Provider
  2. Explicitly marking it to expect the provider_dependencies lint
  3. Only declaring dep3 in dependencies while using dep2 via nested read

The implementation aligns perfectly with the PR's goal of fixing nested ref invocation detection.

Let's verify this is the only test case for nested ref invocations:

✅ Verification successful

Let me check the specific content of the file to see all instances of nested ref.read usage to ensure this is indeed the only test case.


Test case correctly implements nested ref.read detection

The verification confirms this is indeed the only test case for nested ref.read detection in the test suite. The implementation in onFamilyDep provider serves as the dedicated test for this specific scenario, making it a well-isolated test case that properly validates the lint rule's ability to detect dependencies in nested ref.read calls.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other potential nested ref.read test cases
rg -l 'ref\.read.*ref\.read' packages/riverpod_lint_flutter_test/test/

Length of output: 168


Script:

#!/bin/bash
# Get all instances of nested ref.read with surrounding context
rg 'ref\.read.*ref\.read' -B 2 -A 2 packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.dart

Length of output: 226

packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.diff (2)

Line range hint 39-89: LGTM! Comprehensive coverage of dependency update scenarios.

The test cases effectively verify:

  • Empty to non-empty dependency lists
  • Multiple dependencies
  • Dependency removal
  • Family provider dependencies

Line range hint 1-147: Add test cases for nested ref invocations.

While the current test cases thoroughly cover dependency specifications, consider adding specific test cases for nested ref invocations (e.g., ref.read(dep2(ref.read(dep)))) to directly verify the fix mentioned in the PR objectives.

packages/riverpod_lint_flutter_test/test/lints/provider_dependencies/provider_dependencies.g.dart (4)

41-62: LGTM: Hash implementation follows best practices

The hash combining implementation follows standard practices and matches the Dart SDK's approach for generating stable hash codes.


64-189: LGTM: Family provider implementation is well-structured

The implementation follows Riverpod's best practices with:

  • Proper dependency tracking
  • Correct parameter handling
  • Well-implemented equality and hash code generation
  • Appropriate provider element creation

314-316: Deprecation notice is properly documented

The typedef is correctly marked as deprecated with a clear migration path to use Ref instead in version 3.0.


298-312: LGTM: Provider dependencies are correctly declared

The implementation properly declares and tracks dependencies on dep3Provider, including transitive dependencies. This supports the PR's goal of fixing nested ref invocations detection.

Let's verify the dependency declarations:

✅ Verification successful

Let me analyze the results and generate the final response.

The search results confirm:

  1. dep3Provider is properly used in the test files and the generated code, with correct dependency declarations.
  2. The nested ref.read pattern ref.read(dep3Provider(ref.read(dep2Provider))) in the test file is exactly what this PR aims to detect and fix, and the generated code properly handles this by declaring dependencies.

Dependencies are correctly declared and nested ref invocations are properly handled

The implementation in onFamilyDepProvider correctly declares its dependency on dep3Provider and properly tracks transitive dependencies. This is verified by:

  • The presence of dep3Provider in both direct dependencies and transitive dependencies arrays
  • The test file containing the nested ref invocation pattern that this implementation aims to handle
  • No unexpected or incorrect usages of dep3Provider found in the codebase
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that dep3Provider is properly used in the codebase
# and that nested ref invocations are correctly handled

# Search for dep3Provider usage to ensure it's properly integrated
rg "dep3Provider" --type dart

# Search for nested ref.read patterns to verify the fix
rg "ref\.read.*ref\.read" --type dart

Length of output: 5836

packages/riverpod_analyzer_utils_tests/test/ref_invocation_test.dart (3)

414-481: Excellent addition of nested ref.read tests

The new test case thoroughly verifies that nested ref.read invocations with family providers are correctly decoded, including up to the third level of nesting. This ensures proper recursion handling and improves test coverage.


646-713: Effective testing of nested ref.watch invocations

The added test case effectively ensures that nested ref.watch invocations with family providers are properly decoded, up to the third level of nesting. This enhances confidence in the analyzer's ability to handle complex nesting scenarios.


714-758: Comprehensive test for mixed ref.watch and ref.read invocations

This test case successfully verifies that the analyzer can decode a mix of nested ref.watch and ref.read invocations with family providers. It adds valuable coverage for mixed usage scenarios, ensuring correct analysis in complex cases.

- adds 3rd level of nesting to ensure recursion works
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants